Feature/mcp v1#63
Conversation
Add 39 new PHPUnit tests covering the 6 Phase 2 MCP tools. Update ResourceProviderTest to inject WordPressClient mock for live REST data testing. Total: 105 tests, 252 assertions.
Update CURRENT.md and ROADMAP.md to reflect Phase 2 completion. Include src/Rest/ in phpunit.xml source coverage report.
Register the Rest test suite directory so REST controller tests are discovered and run as part of the test suite.
Add ErrorCode class with machine-readable codes, HTTP status mappings, and resolution hints for every MCP error path.
Wire SALTUS_CACHE_*, SALTUS_RATE_LIMIT_* and SALTUS_AUDIT_* environment variables through Config toggles and getters.
Introduce CacheInterface and InMemoryCache with TTL support. WordPressClient::get() checks cache before HTTP calls and stores successful responses. POST/PUT/DELETE invalidate the cache.
Implement RateLimiter with configurable max requests per window. Integrate into Server::handleToolsCall() to throttle API usage before tool dispatch.
Implement AuditEntry value object capturing tool name, arguments, status, duration and timestamps. AuditLogger writes JSON lines to STDERR by default, supports optional file output, and keeps a configurable in-memory circular buffer.
Instantiate Cache, RateLimiter and AuditLogger in the constructor. Replace all inline JSON-RPC error arrays with structured McpError responses via buildError() helper. Add rate limit check before tool dispatch and audit logging to every tool call exit path.
Mark structured error codes, caching, rate limiting and audit trail as completed.
Add ✓ status column to Phase 3 feature table for audit trail, rate limiting, caching layer, structured error codes.
Factory that maintains the canonical list of all MCP tool classes. Both the stdio MCP server and the WordPress-native MCP/Abilities layer use this factory to create ToolProvider instances with identical tool sets.
Replace the private registerTools() method with a single call to ToolFactory::createDefaultProvider(). Removes per-tool use imports and manual tool instantiation.
Replace createMock with createStub since no expectation verification is needed.
Add a 'terms' parameter to ListPosts that accepts taxonomy term filters as {taxonomy_rest_base: [term_id, ...]}. The method resolves taxonomy REST bases from the WordPress API to support both slug and rest_base lookups.
Add AbilityRegistrar that hooks into wp_abilities_api_init to register Saltus MCP tools as WordPress-native abilities. AbilityDefinitionFactory maps ToolInterface definitions to the WordPress ability schema. Include description in ResourceProvider.
Add MCP feature class that hooks into the Saltus feature registration system. Include MCP in the default feature list in Core.php.
Compatibility test covering native ability registration, capability gating, and REST-backed dispatch.
Add get_options, get_rest_base, get_label_singular, get_label_plural, and get_featured_image_label to BaseModel. Add get_associations and is_hierarchical to Taxonomy for REST API consumption.
Extend ModelsController to include rest_base, associations, and hierarchical fields for Taxonomy models. Add test helpers for taxonomy registration and ability functions.
Replace createMock with createStub since no expectation verification is needed.
Document WordPress 7.0 MCP/Abilities integration paths for both native MCP clients and stdio fallback. Add new tools to the available tools table. Update ROADMAP with top priority section and exit criteria. Sync CURRENT.md with recent changes.
- Enable executionOrder=random, beStrictAbout and failOn flags - Add phpunit.xml.dist as distribution configuration
|
/gemini review, big change again, do a deep review |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces version 2.0.0 of the Saltus Framework, featuring a comprehensive integration with the WordPress-native MCP/Abilities API, a new REST API namespace (saltus-framework/v1) with nine routes, and robust runtime controls including transient caching, sliding-window rate limiting, and an audit trail. To support this, the service container has been updated to a two-pass registration model that instantiates REST and MCP providers unconditionally while keeping core admin features gated. Additionally, a complete PHPUnit test suite has been added, and type safety has been hardened across models and asset loaders. Review feedback suggests refining the check_method fallback in ModelsController to use get_name() when get_registration_name is missing, and updating Core.php to fallback to the service class name as the ID when service registration keys are numeric.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| * Safely call a method or access a property on an object, falling back to a default. | ||
| * | ||
| * @param object $target The target object. | ||
| * @param string $method Method name to try first. | ||
| * @param string $default_prop Property name if method does not exist. | ||
| * @param string $default_val Fallback value if neither method nor property exists. | ||
| * @return mixed | ||
| */ | ||
| private function check_method( object $target, string $method, string $default_prop, string $default_val ) { |
There was a problem hiding this comment.
The check_method fallback mechanism is fragile. If a custom model implements the Model interface but does not inherit from BaseModel or define get_registration_name(), its name in the REST response will default to an empty string (''), even though it is guaranteed to have a valid get_name() method by the Model interface contract.
To make this more robust, fallback to get_name() when $method is 'get_registration_name' and the method exists on the target object.
private function check_method( object $target, string $method, string $default_prop, string $default_val ) {
if ( method_exists( $target, $method ) ) {
return $target->{$method}();
}
if ( $method === 'get_registration_name' && method_exists( $target, 'get_name' ) ) {
return $target->get_name();
}
$public_properties = get_object_vars( $target );
return array_key_exists( $default_prop, $public_properties ) ? $public_properties[ $default_prop ] : $default_val;
}| foreach ( $services as $id => $service_class ) { | ||
| $this->service_container->register( $id, $service_class, $dependencies ); | ||
| } |
There was a problem hiding this comment.
If a custom service is appended to the $services array via a filter without an explicit string key, it will be assigned a numeric index. Registering a service with a numeric ID in the container can lead to type errors (if strict types are enforced) or make it impossible to retrieve the service by its expected identifier.
To prevent this, fallback to using the service class name as the ID if the key is numeric.
foreach ( $services as $id => $service_class ) {
$service_id = is_string( $id ) ? $id : $service_class;
$this->service_container->register( $service_id, $service_class, $dependencies );
}1 participant
add mcp for saltus features